Introduce O(n) canonicalization algorithm
#1670
Conversation
|
Per call today @evanlinjin will open a new PR for the 1.0.0-beta milestone that only makes expected breaking changes for |
evanlinjin
force-pushed
the
fix/1665
branch
8 times, most recently
from
5278b81
to
7610b65
Compare
|
On further discussion today at release planning call this PR was moved back into the 1.0 milestone if it can be completed and reviewed in time. If not it will have to wait for a 2.0 milestone because it required breaking changes to chain crate APIs that are exposed in the Alternatively we could remove the following /// Get a reference to the inner [`TxGraph`].
pub fn tx_graph(&self) -> &TxGraph<ConfirmationBlockTime> {
self.indexed_graph.graph()
}
/// Get a reference to the inner [`KeychainTxOutIndex`].
pub fn spk_index(&self) -> &KeychainTxOutIndex<KeychainKind> {
&self.indexed_graph.index
}
/// Get a reference to the inner [`LocalChain`].
pub fn local_chain(&self) -> &LocalChain {
&self.chain
} |
|
@notmandatory how does this solution compare with just giving wallet a major version bump? |
|
Removing the above functions and moving required chain error type to core should allow us to do breaking chain crate api changes without having to do a major wallet crate release. I also don't see why Wallet users need to access the inner chain types directly instead of using higher level functions like But all that said if it's less risky to just keep everything as is and do a 2.0 release in 6 mo or so I'd be fine with that too. |
Change `ChainPosition` to be able to represent transitive anchors and unconfirm-without-last-seen values.
evanlinjin
force-pushed
the
fix/1665
branch
2 times, most recently
from
78c9b0f
to
64733ca
Compare
Previously, the field `TxGraph::anchors` existed because we assumed there was use in having a partially-chronological order of transactions there. Turns out it wasn't used at all. This commit removes anchors from the `txs` field and reworks `anchors` field to be a map of `txid -> set<anchors>`. This is a breaking change since the signature of `all_anchors()` is changed. Update `TxGraph` field docs for `empty_outspends` and `empty_anchors`.
This is an O(n) algorithm to determine the canonical set of txids.
Add `run_until_finished` methods for `TxAncestors` and `TxDescendants`. This is useful for traversing until the internal closure returns `None`. Signatures of `TxAncestors` and `TxDescendants` are changed to enforce generic bounds in the type definition.
`TxGraph::ordered_txs` contains txs ordered by `LastSeenIn` values. To have this change, `insert_anchor` and `insert_seen_at` methods populate the `ordered_txs` field. Generic constaints needed to be tightened as these methods need to be aware of the anchor height to create `LastSeenIn`. The order of `LastSeenIn` variants were wrong and this is now fixed. However, a bug was detected where conflicts with transitively-anchored txs were not detected. The logic of `CanonicalIter` is changed to look at txs with anchors first.
| // let anchor = if height == 0 { | ||
| // ChainPosition::Unconfirmed { last_seen: Some(0) } | ||
| // } else { | ||
| // ChainPosition::Confirmed { | ||
| // anchor: ConfirmationBlockTime { | ||
| // block_id: latest_cp.block_id(), | ||
| // confirmation_time: 0, | ||
| // }, | ||
| // transitively: None, | ||
| // } | ||
| // }; | ||
| // receive_output(wallet, value, anchor) |
Fixes #1665
Replaces #1659
Description
We introduce an
O(n)algorithm to determine the canonical set of transactions inTxGraph.The algorithm traverses graph edges (spends) backwards. We start traversing from transactions with the highest last-seen-in-mempool values as they are most likely to still be in the mempool and not conflict with our confirmed txs.
When a transaction is determined to be canonical, we can also conclude that it's ancestors must be canonical. When a transaction is determined to be non-canonical, we can also conclude that it's descendants are not canonical.
CanonicalIter::canonicalize_by_traversing_backwardsis called on txs ordered byLastSeenIn. Anything determined to be canonical/not-canonical are marked in respective sets so we do not duplicate work.Notes to the reviewers
TODO
Changelog notice
TODO
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes: